Android: add native platform support #2396

finagolfin · 2019-11-01T15:08:05Z

This pull was used to build SPM natively on Android in the Termux app, similar to swiftlang/swift#27167. The first commit consists of compile-time changes that were needed to build SPM itself natively on an Android host, whereas the second commit adds some support for using SPM to build for Android. None of the second commit was needed to run the SPM tests on Android, so any of it can be removed if it interferes with any other plans for Android compilation, though there's currently only two mentions of Android in the SPM source.

This pull was tested against the 10/24 source snapshot and out of 623 test cases run, it reports 72 failing. Most are related to looking for PackageDescription4 and other dependencies of SPM itself, which fails here because it can't find them relative to the test case's temporary path. A couple prints show that that code path isn't hit on linux, but I'm not familiar enough with the SPM source yet to fix that. In the meantime, I thought I'd get the rest of these changes in.

I also had to disable interpreting the SPM manifest to generate the JSON version when bootstrapping, similar to #2363, instead extracting the same command and generating the JSON by compiling the source. Finally, I had to tweak the bootstrap script:

diff --git a/Utilities/bootstrap b/Utilities/bootstrap
index 765b2488..afd9b09c 100755
--- a/Utilities/bootstrap
+++ b/Utilities/bootstrap
@@ -265,6 +265,7 @@ class Target(object):
         if args.foundation_path:
             import_paths.append(args.foundation_path)
             other_args.extend(["-Xcc", "-F" + args.foundation_path])
+            other_args.extend(["-Xcc", "-D__ANDROID_API__=23", "-Xcc", "-U_GNU_SOURCE"])
             import_paths.append(os.path.join(args.foundation_path, "swift"))
         if args.libdispatch_build_dir:
             import_paths.append(os.path.join(args.libdispatch_build_dir, 'lib'))
@@ -583,6 +584,7 @@ class llbuild(object):
                 if platform.system() == 'Linux':
                     link_command.extend(
                         ["-Xlinker", "-rpath=$ORIGIN/../lib/swift/linux"])
+                    link_command.extend(["-Xlinker", "-landroid-spawn"])
                 if self.args.foundation_path:
                     link_command.extend([
                         "-L", self.args.foundation_path,
@@ -1063,7 +1065,15 @@ def main():
     # Compute the build paths.
     if platform.system() == 'Darwin':
         build_target = "x86_64-apple-macosx"
-    elif platform.system() == 'Linux':              
+    elif platform.system() == 'Linux':
+        if 'ANDROID_DATA' in os.environ:
+            if platform.machine().startswith("armv7"):
+                build_target = 'armv7-unknown-linux-androideabi'
+            elif platform.machine() == 'aarch64':
+                build_target = 'aarch64-unknown-linux-android'
+            else:
+                raise SystemExit("ERROR: unsupported Android platform:",platform.machine())
+
         if platform.machine() == 'x86_64':
             build_target = "x86_64-unknown-linux-gnu"
         elif platform.machine() == "i686":
@@ -1075,7 +1085,7 @@ def main():
         elif platform.machine().startswith("armv7"):
             build_target = 'armv7-unknown-linux-gnueabihf'
         elif platform.machine() == 'aarch64':
-            build_target = 'aarch64-unknown-linux'
+            build_target = 'aarch64-unknown-linux-android'
         else:
             raise SystemExit("ERROR: unsupported machine:",platform.machine())
     elif platform.system() == 'Windows':
@@ -1269,6 +1279,7 @@ def main():
         else:
             embed_rpath = "@executable_path/../lib/swift/macosx"
     build_flags.extend(["-Xlinker", "-rpath", "-Xlinker", embed_rpath])
+    build_flags.extend(["-Xlinker", "-landroid-spawn"])
     if args.verbose:
         build_flags.append("-v")
 
@@ -1282,6 +1293,8 @@ def main():
         # Pass -F flag for CoreFoundation.
         build_flags.extend(["-Xswiftc", "-Xcc", "-Xswiftc", "-F{}".format(faketoolchain_includedir)])
 
+        build_flags.extend(["-Xswiftc", "-Xcc", "-Xswiftc", "-D__ANDROID_API__=23"])
+        build_flags.extend(["-Xswiftc", "-Xcc", "-Xswiftc", "-U_GNU_SOURCE"])
         # Pass -I for underlying Dispatch module.
         build_flags.extend(["-Xswiftc", "-I{}".format(os.path.join(faketoolchain_includedir, "dispatch"))])

The two preprocessor definitions are needed because Bionic's strerror_r used to return a different type before API 23; linking against android-spawn because Termux backported and shipped posix_spawn in a separate library; and I'm unsure why that last build_target change was needed, given the prior change for Android (without it, two directories are created with both names and the build errors out). I plan to try cross-compiling SPM itself for Android, so I'll submit a pull for all those build changes later.

Two other minor issues while I'm here:

The build output warns against calling -fmodules-cache-path on clang from the bootstrap script, on both Android and linux:

clang-8: warning: argument unused during compilation: '-fmodules-cache-path=/home/pacman/aur/swift-language/src/build/Ninja-ReleaseAssert/swiftpm-linux-x86_64/.bootstrap/ModuleCache' [-Wunused-command-line-argument]

I'm guessing it's expected that Swift's forked clang is used, but unless it's installed in the system itself, the bootstrap script isn't configured that way. I'm unsure if this was intended or if it matters.

I don't think the bootstrap script compiles in parallel, would be worthwhile to add that to speed up full rebuilds. I saw nice speed gains when Saleem added that for Foundation, build: parallelise build swift-corelibs-foundation#2129.

Sources/PackageDescription4/SupportedPlatforms.swift

Sources/Build/Triple.swift

Sources/PackageModel/Platform.swift

Sources/TSCUtility/FSWatch.swift

aciidgh · 2019-11-01T15:29:55Z

Thanks a lot for working on this! This looks good to me. I think @gmittert is working on adding support for compiling the manifest file for Windows so that should be helpful for Android as well.

aciidgh · 2019-11-01T15:30:03Z

@swift-ci smoke test

finagolfin · 2019-11-01T15:30:07Z

Sources/TSCUtility/FSWatch.swift

+    set.fds_bits = fd_bits
+  #else
+    set.__fds_bits = fd_bits
+  #endif


It'd be nice if I could assign some kind of compile-time symbol reference to each platform's variable name, so that this runtime copying isn't needed.

finagolfin · 2019-11-01T15:31:20Z

Sources/TSCUtility/Platform.swift

+            if localFileSystem.isFile(AbsolutePath("/system/bin/toolbox")) ||
+               localFileSystem.isFile(AbsolutePath("/system/bin/toybox")) {
+                return .android
+            }


This function doesn't seem to be called anywhere, can remove if not needed.

yah let's remove it if it's dead code.

I was wrong, the variable it's used to initialize, currentPlatform, is used in exactly one place. I suppose this might be useful for TSCUtility and llbuild someday too, for which I've created a Termux package, so I'd like to keep this Android support.

Any reason you chose these particular files vs /system/build.prop?

No other reason that I'm familiar with toolbox/toybox, as they're sometimes invoked from the Termux command-line, and not build.prop, which is unreadable without root on my device. If there's a better file that signifies Android, glad to use that instead, just let me know.

OK, that's fine for now, we can always update the check later if we need to.

Thanks for your contribution!

finagolfin · 2019-11-01T15:32:07Z

Sources/TSCUtility/dlopen.swift

@@ -60,8 +60,10 @@ public struct DLOpenFlags: RawRepresentable, OptionSet {
    public static let deepBind: DLOpenFlags = DLOpenFlags(rawValue: 0)
  #else
    public static let first: DLOpenFlags = DLOpenFlags(rawValue: 0)
+  #if !os(Android)
    public static let deepBind: DLOpenFlags = DLOpenFlags(rawValue: RTLD_DEEPBIND)


No RTLD_DEEPBIND on Android, so #ifdef it out here and above.

finagolfin · 2019-11-01T17:32:59Z

Hmm, the failed smoke test indicates that the expected type of mask varies by platform, I'll investigate further tonight and fix it.

jakepetroules

Great start! I have concerns about the deployment target, but other than that, looks good.

Sources/Build/Triple.swift

Sources/PackageModel/Platform.swift

jakepetroules · 2019-11-01T23:26:03Z

Tests/PackageLoadingTests/PackageBuilderTests.swift

@@ -1376,6 +1376,7 @@ class PackageBuilderTests: XCTestCase {
            "ios": "8.0",
            "tvos": "9.0",
            "watchos": "2.0",
+            "android": "7.0",


Had to set to 0.0 just like linux above for this test to pass.

jakepetroules · 2019-11-01T23:26:06Z

Tests/PackageLoadingTests/PackageBuilderTests.swift

@@ -1411,6 +1412,7 @@ class PackageBuilderTests: XCTestCase {
            "linux": "0.0",
            "ios": "8.0",
            "watchos": "2.0",
+            "android": "7.0",


The macOS smoke test for swiftlang/swift-package-manager#2396 fails without this addition.

finagolfin · 2019-11-03T19:11:44Z

Alright, figured out the issue with the type of mask: it's that fd_mask is a UInt in Bionic but Int in glibc, moved mask inside the #ifdef to account for that.

Should be ready to go now, addressed the other issues too. The sourcekit-lsp pull will need to be tested and merged alongside this.

aciidgh · 2019-11-04T14:53:19Z

@swift-ci smoke test

aciidgh · 2019-11-04T17:16:52Z

Please test with the following pull requests:
swiftlang/sourcekit-lsp#186

@swift-ci smoke test

jakepetroules

Looks good, just needs the tweak to the target triple.

Sources/Build/Triple.swift

jakepetroules · 2019-11-04T17:36:25Z

Sources/TSCUtility/Platform.swift

+            if localFileSystem.isFile(AbsolutePath("/system/bin/toolbox")) ||
+               localFileSystem.isFile(AbsolutePath("/system/bin/toybox")) {
+                return .android
+            }


Any reason you chose these particular files vs /system/build.prop?

jakepetroules · 2019-11-04T20:39:55Z

Sources/TSCUtility/Platform.swift

+            if localFileSystem.isFile(AbsolutePath("/system/bin/toolbox")) ||
+               localFileSystem.isFile(AbsolutePath("/system/bin/toybox")) {
+                return .android
+            }


OK, that's fine for now, we can always update the check later if we need to.

Thanks for your contribution!

aciidgh · 2019-11-05T05:31:09Z

Please test with the following pull requests:
swiftlang/sourcekit-lsp#186

@swift-ci smoke test

finagolfin · 2019-11-08T08:09:54Z

Thanks for the quick review and merge, got many of those tests working in the meantime with an adapted version of #2363, will submit that later.

finagolfin requested review from abertelrud, aciidgh and neonichu as code owners November 1, 2019 15:08

aciidgh reviewed Nov 1, 2019

View reviewed changes

Sources/PackageDescription4/SupportedPlatforms.swift Show resolved Hide resolved

finagolfin commented Nov 1, 2019

View reviewed changes

Sources/Build/Triple.swift Outdated Show resolved Hide resolved

finagolfin force-pushed the droid branch from 0f26582 to c59e061 Compare November 1, 2019 15:23

finagolfin commented Nov 1, 2019

View reviewed changes

Sources/PackageModel/Platform.swift Outdated Show resolved Hide resolved

finagolfin commented Nov 1, 2019

View reviewed changes

Sources/TSCUtility/FSWatch.swift Outdated Show resolved Hide resolved

finagolfin commented Nov 1, 2019

View reviewed changes

jakepetroules requested changes Nov 1, 2019

View reviewed changes

finagolfin added a commit to finagolfin/sourcekit-lsp that referenced this pull request Nov 3, 2019

Add new platform for TSCUtility

14f54f4

The macOS smoke test for swiftlang/swift-package-manager#2396 fails without this addition.

finagolfin mentioned this pull request Nov 3, 2019

Add new platform for TSCUtility swiftlang/sourcekit-lsp#186

Merged

finagolfin force-pushed the droid branch 2 times, most recently from 350d86d to 5dd7287 Compare November 3, 2019 18:59

finagolfin force-pushed the droid branch from 5dd7287 to 558f66a Compare November 4, 2019 06:40

aciidgh approved these changes Nov 4, 2019

View reviewed changes

jakepetroules requested changes Nov 4, 2019

View reviewed changes

finagolfin added 2 commits November 5, 2019 00:04

Android: add host platform support

065f0b9

Android: add some build platform support

6b9dcf4

finagolfin force-pushed the droid branch from 558f66a to 6b9dcf4 Compare November 4, 2019 18:37

jakepetroules approved these changes Nov 4, 2019

View reviewed changes

aciidgh merged commit 946f29a into swiftlang:master Nov 8, 2019

finagolfin mentioned this pull request Nov 22, 2019

Android: add bootstrap build support and fix tests #2417

Merged

finagolfin mentioned this pull request Jan 25, 2020

[Docs] [android] Update to the latest NDK 21, fix commands, and remove cruft swiftlang/swift#29439

Merged

Android: add native platform support #2396

Android: add native platform support #2396

Uh oh!

Conversation

finagolfin commented Nov 1, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aciidgh commented Nov 1, 2019

Uh oh!

aciidgh commented Nov 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

finagolfin commented Nov 1, 2019

Uh oh!

jakepetroules left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

finagolfin commented Nov 3, 2019

Uh oh!

aciidgh commented Nov 4, 2019

Uh oh!

aciidgh commented Nov 4, 2019

Uh oh!

jakepetroules left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aciidgh commented Nov 5, 2019

Uh oh!

finagolfin commented Nov 8, 2019

Uh oh!

Uh oh!